Service Line CalcVATAmountLines: count VAT Difference once and clear filters before OnAfterCalcVATAmountLines - #11026
Conversation
…tern CalcVATAmountLines on table 5902 Service Line added the per-line VAT Difference to TotalVATAmount even though Amount Including VAT minus Amount already contains it, so the duplicated amount was settled on the invoice rounding VAT line. Sales Line and Purchase Line accumulate Amount Including VAT minus Amount only; Service Line now does the same. It also left the FindVATAmountLine filters on the var VATAmountLine when OnAfterCalcVATAmountLines fired, which made the Service Statistics VAT specification show a single line. The record is now reset after the rounding block, before the event, on every path. The NA layer copy of the table carries the same code and receives the same changes. Two tests cover each defect in codeunit 134028 and are mirrored into its APAC, ES and IT copies. Fixes microsoft#9102. Re-submission of microsoft/BusinessCentralApps#1913.
Good Sense Reviewer - Round 1Recommendation: AcceptWhat this PR doesThis change aligns Service Line VAT amount-line calculation with the Sales and Purchase implementations. It removes the extra Problem-solution fitFit: Strong The reported problem is that service VAT statistics can show the VAT difference twice and can return a filtered VAT specification after invoice rounding. The diff changes exactly those two service calculation paths and includes tests for both visible failures. SuggestionsNo suggestions. Risk assessment and necessityRisk: The regression surface is Service document VAT statistics when invoice rounding and VAT differences are involved, plus subscribers that read the totals or the VAT amount-line record from the service calculation events. The production change is small and makes Service behavior match the existing Sales and Purchase pattern. Necessity: The change is required to stop an incorrect VAT amount from being assigned to the rounding VAT line and to make all VAT specification lines visible to callers. The scope is limited to the affected W1 and NA Service Line copies, with localized tests mirroring the shared behavior.
|
What & why
CalcVATAmountLineson table 5902 "Service Line" diverges from the Sales Line and Purchase Line implementation in two places, and both show up on Service Statistics as soon as invoice rounding and a manual VAT difference meet:"Amount Including VAT" - Amount + "VAT Difference"per line."Amount Including VAT" - Amountalready contains the VAT difference, so it was counted twice and the excess was settled on the invoice rounding VAT line, which then showed a VAT amount equal to the VAT difference. Sales Line and Purchase Line accumulate"Amount Including VAT" - Amountonly.FindVATAmountLinestayed on thevar VATAmountLinewhenOnAfterCalcVATAmountLinesfired and control returned to the caller. Page 6030 "Service Statistics" then ranTempVATAmountLine.ModifyAll(Modified, false)on one row and handed a filtered set to the VAT Specification subform, so only one VAT line was visible.The fix drops the extra term, which makes the accumulation identical to Sales Line and Purchase Line, and adds
VATAmountLine.Reset()after the rounding block, before the event, so the record is unfiltered on every path. For the record, Sales Line is not fully clean here either:GetVATAmountLineOfMaxAmtresets its filters only on its fall-through branch and leaves them set on the earlyexit. That narrower gap is out of scope for this change. Both layers that carry the procedure are changed:src/Layers/W1and the full table copy insrc/Layers/NA. The two edited regions are byte-identical between the layers before and after the change.Linked work
Fixes #9102
Re-submission of microsoft/BusinessCentralApps#1913 (pilot issues #1562 and #1563), which received a code-owner approval and was closed unmerged when that repository was retired, with a note about internal test failures. Two things differ from that version and are the likely cause of those failures:
"VAT Identifier"on a new VAT Posting Setup directly, which fails in the IT localization, where that field has a table relation to the VAT Identifier table. The tests here go through the codeunit's existingCreateVATPostingSetuphelper, which the IT copy already overrides for exactly that reason.How I validated this
What I tested and the outcome
Two tests were added to codeunit 134028 "ERM Change VAT On VAT Amt Line", and mirrored into its full-replacement copies in the APAC, ES and IT test layers (those copies differ from W1 only in their pre-existing localized lines, verified by diffing each copy against W1 before and after the change).
ServiceCalcVATAmountLinesCountsVATDifferenceOnce: a Service Invoice with an item line (10% VAT) and an invoice rounding G/L line (0% VAT, own VAT Identifier). The VAT amount of the item line is raised by a VAT difference the way the VAT Specification subform and Service Statistics do it (Validate("VAT Amount"),Modified := true,UpdateVATOnLinesfor General and Invoicing), thenCalcVATAmountLinesruns again. It asserts that the total VAT on the VAT Amount Lines equals the VAT on the Service Lines, and that the rounding VAT line stays at 0. Against the unpatched table the residual afterUpdateLinesis exactly the VAT difference, the rounding block adds it to the rounding VAT line, and both assertions fail.ServiceCalcVATAmountLinesReturnsAllLinesAfterRoundingResidual: same document, no VAT difference. A rounding residual is injected throughOnCalcVATAmountLinesOnBeforeUpdateVATAmountLinefrom a manually bound instance of the test codeunit, so the rounding block runs without depending on the rounding arithmetic of a particular localization. It asserts that both VAT Amount Lines are visible on the record handed back to the caller, and that the residual was settled on a VAT Amount Line. With the first fix alone the block still leaves theFindVATAmountLinefilters in place andCount()returns 1, so this test isolates theReset().Neither test has executed yet:
.alpackagesis empty in this repository, so there is no local build, and the pull request build runs only after a maintainer approves the workflow. The pass and fail behaviour above comes from tracing the code paths by hand againstupstream/main, not from a test run.Risk & compatibility
OnCalcVATAmountLinesOnAfterCalcLineTotalson Service Line:TotalVATAmountno longer carries the duplicated VAT difference. That value was wrong; it now matches what the Sales and Purchase publishers pass.OnAfterCalcVATAmountLineson Service Line:VATAmountLinenow arrives unfiltered on every path. Nothing in this repository subscribes to that event.GetVATAmountLineOfMaxAmt.